Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store snapshots using git lfs #253

Merged
merged 5 commits into from
Jan 24, 2025
Merged

Store snapshots using git lfs #253

merged 5 commits into from
Jan 24, 2025

Conversation

tomcur
Copy link
Member

@tomcur tomcur commented Jan 24, 2025

This is based on Vello's LFS setup, including the mechanism that allows easily disabling the tests that rely on LFS, just in case bandwidth runs out.

@tomcur tomcur changed the title Lfs Store snapshots using git lfs Jan 24, 2025
@tomcur
Copy link
Member Author

tomcur commented Jan 24, 2025

Hmm... seems git lfs is having trouble actually pushing the files to GitHub.

@wfdewith
Copy link
Contributor

Interesting, GitHub doesn't show the images in the "Files changed" tab anymore, but if you click "View file", like this, it does show them.

@tomcur
Copy link
Member Author

tomcur commented Jan 24, 2025

Daniel and I think that's because the file moved from the repository to LFS, which GitHub's UI might not know how to deal with. In Vello, the LFS-stored files do have functioning diffs in GitHub's UI. (For example here, scroll to vello_tests/snapshots/big_colr.png).

@wfdewith
Copy link
Contributor

Ah, yeah, it might not be able to show a diff between a non-LFS and LFS file, but that's only a problem for this specific PR.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct to me

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@tomcur tomcur enabled auto-merge January 24, 2025 20:25
@tomcur tomcur disabled auto-merge January 24, 2025 20:25
@tomcur
Copy link
Member Author

tomcur commented Jan 24, 2025

CI is stuck again.

edit: seems it actually was GitHub's YAML parser having trouble parsing null / ~: 930aaea.

Message bump for CI.

Co-authored-by: Daniel McNab <[email protected]>
@tomcur tomcur enabled auto-merge January 24, 2025 20:40
@tomcur tomcur added this pull request to the merge queue Jan 24, 2025
Merged via the queue into main with commit ef0ee99 Jan 24, 2025
21 checks passed
@tomcur tomcur deleted the lfs branch January 24, 2025 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants